Skip to content

chore: remove unused 3-arg McpUsageRecorder.record overload - #32786

Open
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/dead-code/chore-remove-unused-3-arg-mcpusagerecorder-record-2f96b3
Open

chore: remove unused 3-arg McpUsageRecorder.record overload#32786
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/dead-code/chore-remove-unused-3-arg-mcpusagerecorder-record-2f96b3

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 7, 2026

Copy link
Copy Markdown

Describe your changes:

No issue linked — dead code cleanup.

I removed the unused 3-arg McpUsageRecorder.record(String, String, boolean) overload and its dedicated legacy3ArgOverloadOmitsPhase3Fields test, because the only production caller (McpServer) uses the 6-arg form.

The 3-arg overload was the original signature; the single production call site was migrated to the 6-arg overload in the same commit that introduced the 6-arg form, leaving the 3-arg as a delegating shim whose "kept so existing call sites compile unchanged" comment was stale — no production call sites remained. Its only callers were in McpUsageRecorderTest.

The 5 remaining tests that exercised general recording behavior via the shim now call the 6-arg overload with null, null, null (exactly what the removed overload delegated to), so behavior under test is unchanged. The 6-arg record method, its private resolveMcpApp/getDao helpers, and all logging are left intact (live code with a production caller).

Type of change:

  • Improvement

High-level design:

N/A — small change.

Tests:

Use cases covered

  • MCP usage recording still builds and the recorder's existing unit tests pass after removing the unused overload.

Unit tests

  • Updated McpUsageRecorderTest callers to use the 6-arg overload; removed the now-dead legacy3ArgOverloadOmitsPhase3Fields test.
  • mvn test -pl openmetadata-mcp -Dtest=McpUsageRecorderTest passes (6 tests, 0 failures).

Backend integration tests

  • Not applicable (no backend API changes).

Ingestion integration tests

  • Not applicable (no ingestion changes).

Playwright (UI) tests

  • Not applicable (no UI changes).

UI screen recording / screenshots:

Not applicable.

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.
  • For UI changes: I attached a screen recording and/or screenshots above.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

History


Dead Code PRs can be configured here.

@detail-app
detail-app Bot requested a review from harshach September 7, 2026 00:25
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 102b870350cc05335c0444568a87e6cde2fe144e in Playwright run 34071007815, attempt 1.

✅ 557 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 46m 55s

⏱️ Max setup 4m 16s · max shard execution 18m 30s · max shard-job elapsed before upload 21m 29s · reporting 4s

🌐 235.30 requests/attempt · 2.82 app boots/UI scenario · 26.76% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 26.76% (convergence target: at most 15%).
  • Browser traffic was 235.3 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.82 per UI scenario (1630 boots / 578 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 133 0 0 0 0 0
✅ Shard chromium-02 140 0 0 0 0 0
✅ Shard chromium-03 126 0 0 0 0 0
✅ Shard data-asset-rules-01 65 0 0 0 0 0
✅ Shard domain-isolation-01 16 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 11 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant